2800 hi l1c backgrounds#2937
2800 hi l1c backgrounds#2937subagonsouth merged 9 commits intoIMAP-Science-Operations-Center:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Implements IMAP-Hi L1C PSET background-rate computation by deriving background counts directly from L1B direct events, applying background configuration scaling/uncertainties, and wiring the new ancillary input through the CLI and tests.
Changes:
- Added background configuration DataFrame accessor/utilities and an event-filtering iterator for background selections.
- Updated HI L1C PSET generation to load background config and compute isotropic background rates/uncertainties from L1B DE + exposure times.
- Updated CLI ancillary handling and adjusted/rewrote affected tests to match new function signatures.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| imap_processing/hi/hi_l1c.py | Loads background config and computes background counts/rates/uncertainties during PSET generation. |
| imap_processing/hi/utils.py | Adds BackgroundConfig accessor + shared filtering utilities and a background iterator. |
| imap_processing/cli.py | Updates HI L1C PSET processing to require both cal-prod and backgrounds ancillary inputs. |
| imap_processing/tests/hi/test_hi_l1c.py | Updates tests for new hi_l1c/generate_pset_dataset/pset_backgrounds signatures. |
| imap_processing/tests/hi/test_utils.py | Adds coverage for the new background_config DataFrame accessor. |
| imap_processing/tests/hi/conftest.py | Adds fixture path for the background configuration CSV. |
| imap_processing/tests/test_cli.py | Updates CLI test inputs to provide both required HI L1C ancillary files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Call pset_backgrounds with the new signature | ||
| backgrounds_vars = hi_l1c.pset_backgrounds( | ||
| empty_pset.coords, | ||
| background_df, | ||
| hi_l1b_de_dataset, | ||
| hi_goodtimes_dataset, | ||
| exposure_times, | ||
| ) | ||
|
|
||
| assert "background_rates" in backgrounds_vars | ||
| np.testing.assert_array_equal( | ||
| backgrounds_vars["background_rates"].data, | ||
| np.zeros((n_epoch, n_energy, n_cal_prod, n_spin_bins)), | ||
| assert backgrounds_vars["background_rates"].data.shape == ( | ||
| len(empty_pset.coords["epoch"]), | ||
| len(empty_pset.coords["esa_energy_step"]), | ||
| len(empty_pset.coords["calibration_prod"]), | ||
| len(empty_pset.coords["spin_angle_bin"]), | ||
| ) |
There was a problem hiding this comment.
test_pset_backgrounds now only asserts output variable presence and shapes. Since pset_backgrounds() was rewritten to compute counts, apply scaling factors, and propagate uncertainties, this test should include at least one numerical correctness/sanity assertion (e.g., rates constant across spin_angle_bin for isotropy, non-negative rates/uncertainties, and expected behavior when exposure is uniform/zero).
e193b40 to
04f964f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
imap_processing/hi/hi_l1c.py
Outdated
| len(bg_coords["calibration_prod"]), | ||
| len(bg_coords["background_index"]), | ||
| ), | ||
| dtype=np.int32, |
There was a problem hiding this comment.
_compute_background_counts currently allocates background_counts with dtype=np.int32. For long pointings / high event rates, background counts can plausibly exceed the 32-bit range and silently overflow. Prefer using an int64 accumulator (e.g., np.int64) for counts to make the rate/uncertainty computation safe.
| dtype=np.int32, | |
| dtype=np.int64, |
imap_processing/tests/hi/data/l1/imap_hi_90sensor-backgrounds_20240101_v001.csv
Outdated
Show resolved
Hide resolved
220dc38 to
7b0dade
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Create mapping from descriptor to path | ||
| anc_path_dict = { | ||
| dep.descriptor.split("-", 1)[1]: dep.imap_file_paths[ | ||
| 0 | ||
| ].construct_path() | ||
| for dep in anc_dependencies | ||
| } | ||
|
|
||
| # Verify we have both required ancillary files | ||
| if ( | ||
| "cal-prod" not in anc_path_dict | ||
| or "backgrounds" not in anc_path_dict | ||
| ): | ||
| raise ValueError( | ||
| f"Expected exactly one ancillary dependency. Got {anc_paths}" | ||
| f"Missing required ancillary files. Expected 'cal-prod' and " | ||
| f"'backgrounds', got {list(anc_path_dict.keys())}" | ||
| ) |
There was a problem hiding this comment.
anc_path_dict derives keys via dep.descriptor.split("-", 1)[1], which will misbehave or raise IndexError if the descriptor format differs (e.g., missing sensor prefix or additional hyphens). To avoid fragile parsing, prefer a safer normalization (e.g., strip leading "45sensor-"/"90sensor-" when present, otherwise keep descriptor as-is) or map using the file’s parsed name fields, and raise a clear ValueError when required descriptors can’t be determined.
ea5ee40
into
IMAP-Science-Operations-Center:dev
Summary
Implements background rate calculation for HI L1C PSET datasets. Adds
_compute_background_counts()and updatespset_backgrounds()to compute background counts from direct events and convert them to isotropic background rates with uncertainties.Key Changes
imap_processing/hi/hi_l1c.py_compute_background_counts()function: Computes background counts by filtering direct events according to background configuration and binning into 3D array(epoch, calibration_prod, background_index)pset_backgrounds()function: Changed from accepting pre-computed background counts to computing them internally from L1B direct events, then averaging across spin bins to produce isotropic background ratesgenerate_pset_dataset(): Loads background configuration CSV and passes exposure times topset_backgrounds()imap_processing/tests/hi/test_hi_l1c.pytest_pset_counts()andtest_pset_counts_empty_l1b(): Removedbackground_dfparameter andbackground_countsassertionstest_pset_backgrounds(): Updated to match new signature with L1B dataset and goodtimes, added required SPICE fixturestest_generate_pset_dataset_uses_midpoint_time(): Updated mock to returnexposure_timesDataArrayDependencies
Depends on #2935 (background utilities)
Related Issues
Closes: #2800